Bypass f-string tag round-trip for var operation operands - #6798
Conversation
Interpolating a var into an f-string hashes it, permanently registers it in the module-global _global_vars dict (which never evicts - a memory leak, worst under dev hot-reload), and emits a tag that the return expression's __post_init__ regex-decodes back out. For operands of a @var_operation this round-trip is pure overhead (~30-40% of op construction): the operation merges operand VarData directly from _args. var_operation now suppresses tagging on its operands (ref-counted, so nested operations on the same var can't clear an outer suppression early) while the body runs. Vars created inside the body still tag normally, so their VarData keeps flowing through the return expression. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Jy8uHH11KircGa2MbTrR8g
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Jy8uHH11KircGa2MbTrR8g
Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
Merging this PR will not alter performance
Comparing 
Footnotes
|
Greptile SummaryThis PR fixes a real memory leak where every
Confidence Score: 5/5The change is narrowly scoped to the The ref-counting logic is correct, the Files Needing Attention: packages/reflex-base/src/reflex_base/vars/base.py — specifically the ordering of the increment loop relative to the
|
| Filename | Overview |
|---|---|
| packages/reflex-base/src/reflex_base/vars/base.py | Adds ref-counted _format_without_tagging suppression via direct __dict__ mutation on operands in var_operation wrapper, and a fast-path in Var.__format__; increment loop runs before the try block (known issue in previous thread, no new issues found here) |
| tests/units/vars/test_base.py | Adds four targeted regression tests covering no _global_vars growth, flag cleanup, operand and body-created var VarData reachability, and formatting still tagging outside an operation |
| packages/reflex-base/news/6798.performance.md | Changelog entry accurately describes the memory-leak fix and CPU improvement |
Reviews (2): Last reviewed commit: "Rename news fragment to live PR number a..." | Re-trigger Greptile
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5f75b20c48
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if self.__dict__.get("_format_without_tagging"): | ||
| return str(self) |
There was a problem hiding this comment.
When the same Var is shared across threads, this instance-dict flag changes __format__ process-wide while an operation body is running. A concurrent f"{var}" therefore returns the raw expression without its tag, so a Var constructed from that string loses the original imports/hooks; overlapping operations can also race the non-atomic reference-count updates and leave the flag set or raise during cleanup. Use thread/task-local suppression rather than mutating the shared immutable operand.
Useful? React with 👍 / 👎.
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| for operand in operands: | ||
| operand_dict = operand.__dict__ | ||
| operand_dict["_format_without_tagging"] = ( # pyright: ignore[reportIndexIssue] | ||
| operand_dict.get("_format_without_tagging", 0) + 1 | ||
| ) | ||
| try: | ||
| return_var = func(*args_vars.values(), **kwargs_vars) # pyright: ignore [reportCallIssue] | ||
| finally: | ||
| for operand in operands: | ||
| operand_dict = operand.__dict__ | ||
| remaining = operand_dict["_format_without_tagging"] - 1 | ||
| if remaining: | ||
| operand_dict["_format_without_tagging"] = remaining # pyright: ignore[reportIndexIssue] | ||
| else: | ||
| del operand_dict["_format_without_tagging"] # pyright: ignore[reportIndexIssue] |
There was a problem hiding this comment.
The increment loop runs before the try block, so if it raises mid-iteration (e.g., an operand whose __dict__ is inaccessible), any operands already incremented will have _format_without_tagging stuck at a non-zero value permanently — causing them to silently bypass tagging in all future uses. This can happen today when LiteralVar.create(range_value) returns None (see LiteralVar.create line ~1802), placing None in operands; the first valid-var operand gets incremented, then None.__dict__ raises before the try, so the finally never runs. Moving the increment loop inside try (and using .get(..., 0) in the decrement to tolerate partial increments) would guarantee cleanup regardless of which step fails.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
3 issues found across 3 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/reflex-base/src/reflex_base/vars/base.py">
<violation number="1" location="packages/reflex-base/src/reflex_base/vars/base.py:937">
P2: Supported `NumberVar` format specs inside a `var_operation` still grow `_global_vars`: `NumberVar.__format__` returns early with a formatted nested operation before this suppression check is reached. Applying the suppression-aware raw return in the number-format branches would avoid retaining one intermediate Var per operation.</violation>
<violation number="2" location="packages/reflex-base/src/reflex_base/vars/base.py:937">
P2: Operand f-strings passed through `LiteralVar.create` inside a custom operation now become literal text instead of expressions containing the operand, because this removes the marker consumed by `LiteralStringVar.create`. Consider limiting the raw interpolation path to construction of `CustomVarOperationReturn.js_expression` or preserving a non-global marker that the literal parser can still decode.</violation>
<violation number="3" location="packages/reflex-base/src/reflex_base/vars/base.py:1961">
P2: Formatting a shared Var in another thread can silently lose its tag while any `var_operation` using that Var is running, and concurrent refcount updates can leave stale suppression state. A context-local suppression set (for example, `ContextVar` with token reset) would isolate nested operations without mutating otherwise immutable shared Vars.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Fix all with cubic | Re-trigger cubic
| # normally and keep contributing VarData via the return expression. | ||
| for operand in operands: | ||
| operand_dict = operand.__dict__ | ||
| operand_dict["_format_without_tagging"] = ( # pyright: ignore[reportIndexIssue] |
There was a problem hiding this comment.
P2: Formatting a shared Var in another thread can silently lose its tag while any var_operation using that Var is running, and concurrent refcount updates can leave stale suppression state. A context-local suppression set (for example, ContextVar with token reset) would isolate nested operations without mutating otherwise immutable shared Vars.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/reflex-base/src/reflex_base/vars/base.py, line 1961:
<comment>Formatting a shared Var in another thread can silently lose its tag while any `var_operation` using that Var is running, and concurrent refcount updates can leave stale suppression state. A context-local suppression set (for example, `ContextVar` with token reset) would isolate nested operations without mutating otherwise immutable shared Vars.</comment>
<file context>
@@ -1941,10 +1948,34 @@ def wrapper(*args: P.args, **kwargs: P.kwargs) -> Var[T]:
+ # normally and keep contributing VarData via the return expression.
+ for operand in operands:
+ operand_dict = operand.__dict__
+ operand_dict["_format_without_tagging"] = ( # pyright: ignore[reportIndexIssue]
+ operand_dict.get("_format_without_tagging", 0) + 1
+ )
</file context>
| # raw JS expression: their VarData flows through the operation's | ||
| # ``_args``, so the tag round-trip (and its permanent ``_global_vars`` | ||
| # entry) is pure overhead there. See ``var_operation``. | ||
| if self.__dict__.get("_format_without_tagging"): |
There was a problem hiding this comment.
P2: Supported NumberVar format specs inside a var_operation still grow _global_vars: NumberVar.__format__ returns early with a formatted nested operation before this suppression check is reached. Applying the suppression-aware raw return in the number-format branches would avoid retaining one intermediate Var per operation.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/reflex-base/src/reflex_base/vars/base.py, line 937:
<comment>Supported `NumberVar` format specs inside a `var_operation` still grow `_global_vars`: `NumberVar.__format__` returns early with a formatted nested operation before this suppression check is reached. Applying the suppression-aware raw return in the number-format branches would avoid retaining one intermediate Var per operation.</comment>
<file context>
@@ -930,6 +930,13 @@ def __format__(self, format_spec: str) -> str:
+ # raw JS expression: their VarData flows through the operation's
+ # ``_args``, so the tag round-trip (and its permanent ``_global_vars``
+ # entry) is pure overhead there. See ``var_operation``.
+ if self.__dict__.get("_format_without_tagging"):
+ return str(self)
+
</file context>
| # raw JS expression: their VarData flows through the operation's | ||
| # ``_args``, so the tag round-trip (and its permanent ``_global_vars`` | ||
| # entry) is pure overhead there. See ``var_operation``. | ||
| if self.__dict__.get("_format_without_tagging"): |
There was a problem hiding this comment.
P2: Operand f-strings passed through LiteralVar.create inside a custom operation now become literal text instead of expressions containing the operand, because this removes the marker consumed by LiteralStringVar.create. Consider limiting the raw interpolation path to construction of CustomVarOperationReturn.js_expression or preserving a non-global marker that the literal parser can still decode.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/reflex-base/src/reflex_base/vars/base.py, line 937:
<comment>Operand f-strings passed through `LiteralVar.create` inside a custom operation now become literal text instead of expressions containing the operand, because this removes the marker consumed by `LiteralStringVar.create`. Consider limiting the raw interpolation path to construction of `CustomVarOperationReturn.js_expression` or preserving a non-global marker that the literal parser can still decode.</comment>
<file context>
@@ -930,6 +930,13 @@ def __format__(self, format_spec: str) -> str:
+ # raw JS expression: their VarData flows through the operation's
+ # ``_args``, so the tag round-trip (and its permanent ``_global_vars``
+ # entry) is pure overhead there. See ``var_operation``.
+ if self.__dict__.get("_format_without_tagging"):
+ return str(self)
+
</file context>
4 participants
All Submissions:
Type of change
Changes To Core Features:
What this does
Interpolating a
Varinto an f-string (Var.__format__) hashes the var, permanently registers it in the module-global_global_varsdict (which never evicts — a real memory leak, worst under dev hot-reload since entries and theirGLOBAL_CACHEvalues survive GC), and emits a<reflex-var>tag that the constructed expression's__post_init__then regex-decodes back out. Every@var_operation(all arithmetic, comparisons, string/array/object ops) pays this round-trip for each operand — and for operands it is pure overhead, because theirVarDataalready reaches the operation throughCustomVarOperation._args.Fix
var_operationnow suppresses tagging on its operand vars while the body runs:finally;VarDatakeeps flowing through the return expression (covered by a dedicated test);Measured results
Memory leak (the primary win). Constructing 1000 operations with distinct operands grows
_global_varsby 1001 entries before this change, 0 after. This growth is unbounded over the life of the process and invisible to CodSpeed (no memory instrument on these benchmarks).Micro-level CPU. Local A/B against the base commit (same machine,
timeit, best of 5): constructinga + bdrops from 18.17 µs/op to 13.52 µs/op (~25% faster).CodSpeed benchmark suite: no measurable change. Head vs base shows +0.1% total impact with all 27 benchmarks "Unchanged" (e.g.
test_console_log371.7 µs → 372.2 µs,test_evaluate_page[_complicated_page]27.3 ms → 27.3 ms). The base run's flamegraph explains why: evaluate/compile time is dominated byComponent._post_init(~61% of total),LiteralVar._create_literal_var(~17%), and_isinstancechecks — var-operation construction is too thin a slice for a ~25% cut of it to move the suite. The benchmark-visible motivation for this PR is the leak fix; the CPU reduction is real but only at the micro level.Verification
_global_varsdoes not grow when constructing an operation while operandVarDatastill reaches the merged result; the suppression flag does not persist after the op; body-created vars keep contributingVarData; formatting outside an operation still registers/tags as before.Part of a compiler-performance series; tracked in Linear as ENG-10104.